Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement quicknode geth vs reth output comparison #775

Conversation

jsy1218
Copy link
Member

@jsy1218 jsy1218 commented Jul 2, 2024

Paradigm RETH just released production read 1.0.0. QuickNode is ready for us to consume the production-ready dedicated RETH cluster as well. From Uniswap perspective, the migration from GETH to RETH boils to two considerations:

  1. Can we rely on the RPC provider to do automatic failover for us? Meanwhile QuickNode could, Uniswap has no way to know when that happens. Also RETH is newly productionized, it's better if the clients do the output comparisons ourselves. Hence this PR aims to compare the GETH vs RETH RPC responses ourselves.
  2. Can we confidently live migrate from GETH to RETH at Uniswap scale? Since RETH is only ready on L1, we are only speaking the Uniswap RPC volume on L1:
    a. eth_call is about 1.4k RPS
    b. eth_getBlockNumber is about 345 RPS
    c. ethers js v5.5 style send, which can be eth_call/eth_getBlockNumber/eth_feeHistory/eth_estimateGas RPC. Those calls are in additional to the RPC calls in (a) and (b):
Screenshot 2024-07-15 at 1 45 55 PM

We have done the Perf testing using the Paradigm Flood, at above mentioned scale, and can see QuickNode RETH can cut QuickNode GETH latencies by half in P50, P90, and P99:
Screenshot 2024-07-15 at 1 50 30 PM

With this perf testing, we will implement a live migration in a separate PR, such that it allows us to migrate from GETH to RETH at designated percent as we want, i.e. 100% GETH 0% RETH -> 99% GETH 1% RETH -> 95% GETH 5% RETH -> ... -> 0% GETH 100% RETH.

Client output comparison implementation correctness wise, I've covered with complete unit test coverage, as well as run npm run test:e2e against my local AWS routing-api endpoint:
Screenshot 2024-07-15 at 2 04 39 PM

Due to low traffic volume, I can only verify the basic RPC outputs matching on my local. I will have to merge to prod to see the comparison at a subset of prod traffic volume (sampling at 5%).

@jsy1218 jsy1218 changed the title feat: implement quicknode geth vs reth output comparison [wip] feat: implement quicknode geth vs reth output comparison Jul 2, 2024
Copy link
Member Author

jsy1218 commented Jul 2, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @jsy1218 and the rest of your teammates on Graphite Graphite

@jsy1218 jsy1218 marked this pull request as ready for review July 2, 2024 01:23
@graphite-app graphite-app bot requested review from cgkol, mikeki, a team, xrsv and uni-guillaume July 2, 2024 01:26
Copy link

graphite-app bot commented Jul 2, 2024

Graphite Automations

"Request reviewers once CI passes on routing-api repo" took an action on this PR • (07/02/24)

6 reviewers were added and 1 assignee was added to this PR based on 's automation.

Copy link
Collaborator

@cgkol cgkol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets do it!

Copy link
Contributor

@mikeki mikeki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early comments/questions

lib/rpc/SingleJsonRpcProvider.ts Show resolved Hide resolved
lib/rpc/UniJsonRpcProvider.ts Outdated Show resolved Hide resolved
@xrsv
Copy link
Contributor

xrsv commented Jul 2, 2024

@jsy1218 just curious what are the benefits switching to RETH? is it supposed to be faster?

@jsy1218
Copy link
Member Author

jsy1218 commented Jul 2, 2024

@jsy1218 just curious what are the benefits switching to RETH? is it supposed to be faster?

@xrsv Short-term, we benefit from a faster quote (presumably RETH output has the same correctness as GETH). Long-term, the entire ethereum ecosystem benefit from a higher throughput, better performance for full/archive node syncing, better client diversity from GETH, etc. I see Uniswap switching from GETH to RETH for high up-time full-node use case will bring positive effect to those long-term benefits overtime.

@jsy1218 jsy1218 force-pushed the jsy1218/route-168-implement-quicknode-geth-vs-reth-output-comparison branch from 283d9e7 to d66b28e Compare July 3, 2024 20:05
@jsy1218 jsy1218 force-pushed the jsy1218/route-168-implement-quicknode-geth-vs-reth-output-comparison branch from d66b28e to e393fff Compare July 15, 2024 10:48
@jsy1218 jsy1218 changed the title [wip] feat: implement quicknode geth vs reth output comparison feat: implement quicknode geth vs reth output comparison Jul 15, 2024
@jsy1218
Copy link
Member Author

jsy1218 commented Jul 15, 2024

@mikeki @cgkol I'm going to merge this PR, as I've written the complete unit test coverage. I'm slightly uncertain about the ethers v5.5 style send output comparison, hence I want to ship to prod, and see if there's any unexpected output mismatch, due to any implementation glitch in this PR.

@jsy1218 jsy1218 merged commit 68282c3 into main Jul 15, 2024
5 checks passed
@jsy1218 jsy1218 deleted the jsy1218/route-168-implement-quicknode-geth-vs-reth-output-comparison branch July 15, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants